Skip to content

chart(feature): add dnsPolicy and dnsConfig support#3126

Merged
VietND96 merged 4 commits intoSeleniumHQ:trunkfrom
DrFaust92:add-dns-config
Apr 27, 2026
Merged

chart(feature): add dnsPolicy and dnsConfig support#3126
VietND96 merged 4 commits intoSeleniumHQ:trunkfrom
DrFaust92:add-dns-config

Conversation

@DrFaust92
Copy link
Copy Markdown
Contributor

@DrFaust92 DrFaust92 commented Apr 26, 2026

Summary

  • Expose pod dnsPolicy and dnsConfig across all Selenium Grid components: hub, router, distributor, event bus, session map, session queue, chrome/edge/firefox/relay nodes (including KEDA ScaledJob pods), and the videoManager file-browser.
  • Configurable globally at global.seleniumGrid.{dnsPolicy,dnsConfig} with per-component overrides, mirroring the existing affinity / topologySpreadConstraints pattern.
  • Defaults are non-disruptive: both fields are unset (Kubernetes defaults) unless explicitly configured.

Test plan

  • helm lint passes (default values and with overrides).
  • helm template with default values emits no dnsPolicy / dnsConfig keys.
  • helm template with global.seleniumGrid.dnsPolicy=ClusterFirst and global.seleniumGrid.dnsConfig.nameservers[0]=8.8.8.8 propagates to all 11 components.
  • Per-component overrides (e.g. components.router.dnsPolicy=Default, chromeNode.dnsConfig.options[0]={name: ndots, value: 3}) take precedence over globals.
  • KEDA ScaledJob mode (autoscaling.scalingType=job) renders dnsPolicy / dnsConfig inside jobTargetRef.template.spec for chrome/edge/firefox/relay nodes.

🤖 Generated with Claude Code

@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add revisionHistoryLimit and dnsConfig support to Selenium Grid Helm chart

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add revisionHistoryLimit support across all Selenium Grid components with global default of 10
  and per-component overrides
• Add dnsPolicy and dnsConfig support for pod DNS configuration at global and component levels
• Update helper template to support DNS settings in pod specs including KEDA ScaledJob pods
• Extend values.yaml with new configuration options for all components (hub, router, distributor,
  event bus, session map, session queue, and browser nodes)
Diagram
flowchart LR
  A["Global Config<br/>revisionHistoryLimit<br/>dnsPolicy<br/>dnsConfig"] -->|"defaults"| B["Component Deployments<br/>hub, router, distributor<br/>eventBus, sessionMap<br/>sessionQueue"]
  A -->|"defaults"| C["Browser Nodes<br/>chrome, firefox, edge"]
  B -->|"per-component<br/>overrides"| D["Pod Specs<br/>with DNS & revision settings"]
  C -->|"per-component<br/>overrides"| D
  E["Helper Template<br/>_helpers.tpl"] -->|"renders"| D
Loading

Grey Divider

File Changes

1. charts/selenium-grid/templates/_helpers.tpl ✨ Enhancement +7/-0

Add dnsPolicy and dnsConfig rendering to pod template helper

charts/selenium-grid/templates/_helpers.tpl


2. charts/selenium-grid/templates/hub-deployment.yaml ✨ Enhancement +9/-1

Add revisionHistoryLimit and DNS configuration support

charts/selenium-grid/templates/hub-deployment.yaml


3. charts/selenium-grid/templates/router-deployment.yaml ✨ Enhancement +9/-1

Add revisionHistoryLimit and DNS configuration support

charts/selenium-grid/templates/router-deployment.yaml


View more (8)
4. charts/selenium-grid/templates/distributor-deployment.yaml ✨ Enhancement +9/-1

Add revisionHistoryLimit and DNS configuration support

charts/selenium-grid/templates/distributor-deployment.yaml


5. charts/selenium-grid/templates/event-bus-deployment.yaml ✨ Enhancement +9/-1

Add revisionHistoryLimit and DNS configuration support

charts/selenium-grid/templates/event-bus-deployment.yaml


6. charts/selenium-grid/templates/session-map-deployment.yaml ✨ Enhancement +9/-1

Add revisionHistoryLimit and DNS configuration support

charts/selenium-grid/templates/session-map-deployment.yaml


7. charts/selenium-grid/templates/session-queue-deployment.yaml ✨ Enhancement +9/-1

Add revisionHistoryLimit and DNS configuration support

charts/selenium-grid/templates/session-queue-deployment.yaml


8. charts/selenium-grid/templates/chrome-node-deployment.yaml ✨ Enhancement +1/-0

Add revisionHistoryLimit configuration to chrome node

charts/selenium-grid/templates/chrome-node-deployment.yaml


9. charts/selenium-grid/templates/firefox-node-deployment.yaml ✨ Enhancement +1/-0

Add revisionHistoryLimit configuration to firefox node

charts/selenium-grid/templates/firefox-node-deployment.yaml


10. charts/selenium-grid/templates/edge-node-deployment.yaml ✨ Enhancement +1/-0

Add revisionHistoryLimit configuration to edge node

charts/selenium-grid/templates/edge-node-deployment.yaml


11. charts/selenium-grid/values.yaml ⚙️ Configuration changes +60/-0

Add global and per-component DNS and revision history configuration

charts/selenium-grid/values.yaml


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review Bot commented Apr 26, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Zero revisionHistoryLimit ignored 🐞 Bug ≡ Correctness
Description
Per-component revisionHistoryLimit: 0 will not take effect because Helm’s default treats 0 as
“empty” and falls back to the global value. This makes it impossible to configure a single
Deployment to retain zero ReplicaSets when the global value is non-zero.
Code

charts/selenium-grid/templates/hub-deployment.yaml[19]

+  revisionHistoryLimit: {{ default .Values.global.seleniumGrid.revisionHistoryLimit .Values.hub.revisionHistoryLimit }}
Evidence
The new templates render revisionHistoryLimit via default global component, and values.yaml
exposes per-component overrides; with Helm/Sprig, default considers 0 empty, so an explicit 0
override is dropped and the global value is rendered instead.

charts/selenium-grid/templates/hub-deployment.yaml[17-21]
charts/selenium-grid/values.yaml[660-665]
Best Practice: Helm/Sprig template functions (default)

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`revisionHistoryLimit` is rendered using Helm/Sprig `default`, which treats numeric `0` as empty. This prevents users from setting a per-component `revisionHistoryLimit: 0` (it will silently fall back to the global value).

### Issue Context
This pattern is duplicated across all Deployment templates. A robust fix must distinguish between `nil`/unset and `0`.

### Fix Focus Areas
- charts/selenium-grid/templates/hub-deployment.yaml[17-21]
- charts/selenium-grid/templates/router-deployment.yaml[14-17]
- charts/selenium-grid/templates/distributor-deployment.yaml[14-17]
- charts/selenium-grid/templates/event-bus-deployment.yaml[14-17]
- charts/selenium-grid/templates/session-map-deployment.yaml[14-17]
- charts/selenium-grid/templates/session-queue-deployment.yaml[14-17]
- charts/selenium-grid/templates/chrome-node-deployment.yaml[17-22]
- charts/selenium-grid/templates/firefox-node-deployment.yaml[17-22]
- charts/selenium-grid/templates/edge-node-deployment.yaml[17-22]

### Implementation guidance
Replace `default` with an explicit nil-check-based selection, e.g.:
- Start with `$rev := .Values.global.seleniumGrid.revisionHistoryLimit`
- If the component value is not `nil`, use it (even if it is `0`)
- Render `revisionHistoryLimit: {{ $rev }}`

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Global DNS not clearable 🐞 Bug ≡ Correctness
Description
When global.seleniumGrid.dnsPolicy/dnsConfig is set, a component cannot override it back to
“unset” because empty-string/empty-map overrides are treated as empty and default re-applies the
global value. This prevents per-component opt-out of global DNS settings.
Code

charts/selenium-grid/templates/_helpers.tpl[R517-523]

+  {{- if or $.Values.global.seleniumGrid.dnsPolicy .node.dnsPolicy }}
+    dnsPolicy: {{ default $.Values.global.seleniumGrid.dnsPolicy .node.dnsPolicy }}
+  {{- end }}
+  {{- if or $.Values.global.seleniumGrid.dnsConfig .node.dnsConfig }}
+    {{- $dnsConfigYaml := default $.Values.global.seleniumGrid.dnsConfig .node.dnsConfig }}
+    dnsConfig: {{- toYaml $dnsConfigYaml | nindent 6 }}
+  {{- end }}
Evidence
The new DNS blocks are gated by truthiness (if or global component) and choose values via `default
global component; with Go template emptiness rules, "" and {}` are empty, so they cannot be used
to override a non-empty global value to empty/unset.

charts/selenium-grid/templates/_helpers.tpl[517-523]
charts/selenium-grid/values.yaml[30-35]
charts/selenium-grid/values.yaml[660-665]
Best Practice: Helm/Go template truthiness + Sprig default

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Current DNS rendering (`if or ...` + `default global component`) makes it impossible to clear a globally-configured `dnsPolicy` / `dnsConfig` for a specific component.

### Issue Context
- Values currently default to `dnsPolicy: ""` and `dnsConfig: {}` (global and per-component).
- Templates rely on truthiness and `default`, which collapses "unset" and "explicitly empty" into the same behavior.

### Fix Focus Areas
- charts/selenium-grid/templates/_helpers.tpl[517-523]
- charts/selenium-grid/templates/hub-deployment.yaml[175-181]
- charts/selenium-grid/templates/router-deployment.yaml[159-165]
- charts/selenium-grid/templates/distributor-deployment.yaml[175-181]
- charts/selenium-grid/templates/event-bus-deployment.yaml[86-92]
- charts/selenium-grid/templates/session-map-deployment.yaml[85-91]
- charts/selenium-grid/templates/session-queue-deployment.yaml[82-88]
- charts/selenium-grid/values.yaml[30-35]
- charts/selenium-grid/values.yaml[346-351]
- charts/selenium-grid/values.yaml[660-665]

### Implementation guidance
1) Change defaults for `dnsPolicy` and `dnsConfig` (global + per-component) from `""` / `{}` to `null` (YAML empty value, e.g. `dnsPolicy:` and `dnsConfig:`) so templates can differentiate unset vs explicitly empty.
2) Replace `if or ...` + `default ...` with logic that:
  - Starts from global value
  - If component value is non-nil, uses it (even if empty)
  - Only renders the key when the selected value is non-empty
This enables explicit opt-out by setting component `dnsPolicy: ""` or `dnsConfig: {}` while preserving inheritance when the component value is unset (null).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread charts/selenium-grid/templates/hub-deployment.yaml Outdated
Expose pod dnsPolicy and dnsConfig across all Selenium Grid components
(hub, router, distributor, event bus, session map, session queue,
chrome/edge/firefox/relay nodes including KEDA ScaledJob pods, and the
videoManager file-browser). Configurable globally via
global.seleniumGrid.{dnsPolicy,dnsConfig} with per-component overrides,
mirroring the existing affinity / topologySpreadConstraints pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@DrFaust92 DrFaust92 changed the title chart(feature): add support for revisionHistoryLimit and dnsConfig chart(feature): add dnsPolicy and dnsConfig support Apr 26, 2026
DrFaust92 and others added 3 commits April 25, 2026 21:26
Run helm-docs against values.yaml to surface the new
dnsPolicy/dnsConfig entries added in eb1bdae across global,
components.{router,distributor,eventBus,sessionMap,sessionQueue},
hub, chrome/firefox/edge/relay nodes, and videoManager.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Re-run helm-docs with --skip-version-footer to match the prior
CONFIGURATION.md convention (no autogenerated trailer).

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
helm-docs --skip-version-footer strips the footer plus the blank
line that preceded it. Re-add the trailing newline to match the
prior file shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@VietND96 VietND96 merged commit 312af17 into SeleniumHQ:trunk Apr 27, 2026
55 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants